From 3390c7554733fa360bc8aaab2fd5e2b273bbed34 Mon Sep 17 00:00:00 2001 From: UNITRONIX <36471318+UNITRONIX@users.noreply.github.com> Date: Mon, 16 Mar 2026 23:00:14 +0100 Subject: [PATCH] Fix relay empty-UUID & Docker apk retries Generate a UUID when RequestRelay/RelayResponse messages contain an empty uuid to prevent relay pairing failures (updates in signal/handler.go: handleRequestRelay, handleRequestRelayTCP, handleRelayResponseForward). Add validation in config.GetRelayServers to reject obviously invalid/too-short hosts (prevents relay entries like "a:21117"). Add retry wrappers to apk add commands in Dockerfile, Dockerfile.server, and Dockerfile.console to work around transient DNS failures during image builds. Update changelog (.github/copilot-instructions.md) with these fixes and related notes. --- .github/copilot-instructions.md | 10 +++++++- Dockerfile | 16 +++++++++--- Dockerfile.console | 5 ++-- Dockerfile.server | 4 ++- betterdesk-server/config/config.go | 11 ++++++++ betterdesk-server/signal/handler.go | 40 ++++++++++++++++++++++++----- 6 files changed, 72 insertions(+), 14 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index f623b5b3..3781d026 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -467,6 +467,12 @@ sudo apt-get install -y build-essential libsqlite3-dev pkg-config libssl-dev git 100. [x] **Password modal plaintext (Issue #60)**: `modal.js` `prompt()` only checked `options.type`, but `users.js` passed `inputType: 'password'`. Fixed modal to check both `options.type` and `options.inputType`. 101. [x] **Closed 12 resolved GitHub issues**: #59, #56, #52, #28, #54, #58, #19, #53, #61, #60, #57, #48 — all verified and closed with detailed resolution comments. +#### Go Server — Empty UUID & Relay Fix (Phase 19) ✅ COMPLETED 2026-03-18 +102. [x] **Root cause: Empty UUID in relay (Issues #58, #63, #64)**: When hole-punch fails, RustDesk client sends `RequestRelay{uuid=""}` because `PunchHoleResponse` protobuf has no `uuid` field. Signal server propagated empty UUID to target and relay → relay rejected both connections. Fixed `handleRequestRelay()` (UDP) and `handleRequestRelayTCP()` (TCP) to generate `uuid.New().String()` when `msg.Uuid` is empty. +103. [x] **handleRelayResponseForward safety**: Added empty UUID warning + generation in `handleRelayResponseForward()` for target-initiated relay flow (last-resort safety net). +104. [x] **Relay server address validation**: `GetRelayServers()` in `config/config.go` now rejects entries with host < 2 characters (prevents `relay=a:21117` from invalid config). +105. [x] **Docker DNS resilience (Issue #62)**: Added retry logic (`|| { sleep 2 && apk add ...; }`) to all `apk add --no-cache` commands in `Dockerfile`, `Dockerfile.server`, and `Dockerfile.console` for transient DNS failures on AlmaLinux/CentOS. + --- ## 🔄 System Statusu v3.0 @@ -641,6 +647,8 @@ Pełna dokumentacja budowania: [BUILD_GUIDE.md](../docs/BUILD_GUIDE.md) 29. ~~**Address Book sync fails (Issue #57)**~~ ✅ ROZWIĄZANE - Go server `/api/ab` endpoints were stubs returning empty data. Added `address_books` table + full GET/POST handlers for `/api/ab`, `/api/ab/personal`, `/api/ab/tags` with SQLite + PostgreSQL support — Phase 18 30. ~~**Settings password "password is required" (Issue #60)**~~ ✅ ROZWIĄZANE - `settings.js` sent snake_case fields, `auth.routes.js` expected camelCase. Fixed field names + added missing `confirmPassword` — Phase 18 31. ~~**Password modal plaintext (Issue #60)**~~ ✅ ROZWIĄZANE - `modal.js` prompt checked `options.type` but `users.js` passed `inputType`. Fixed to check both — Phase 18 +32. ~~**Empty UUID in relay causes all WAN connections to fail (Issues #58, #63, #64)**~~ ✅ ROZWIĄZANE - `PunchHoleResponse` has no `uuid` field, so when hole-punch fails, client sends `RequestRelay{uuid=""}`. Signal server now generates `uuid.New().String()` when empty in both `handleRequestRelay()` (UDP) and `handleRequestRelayTCP()` (TCP). Relay address validation rejects `host < 2 chars` (prevents `relay=a:21117`) — Phase 19 +33. ~~**Docker DNS failures during build (Issue #62)**~~ ✅ ROZWIĄZANE - Added retry logic to all `apk add --no-cache` commands in Dockerfile, Dockerfile.server, Dockerfile.console — Phase 19 --- @@ -730,4 +738,4 @@ All code changes MUST include a security review as part of the implementation pr --- -*Ostatnia aktualizacja: 2026-03-17 (Go Server Address Book & Issue Fixes — Phase 18) przez GitHub Copilot* +*Ostatnia aktualizacja: 2026-03-18 (Go Server Empty UUID & Relay Fix — Phase 19) przez GitHub Copilot* diff --git a/Dockerfile b/Dockerfile index 7ea65f68..69a5125a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,7 +19,8 @@ # ============= Stage 1: Build Go server ============= FROM golang:1.25-alpine AS go-builder -RUN apk add --no-cache git +# Retry apk in case of transient DNS failures (common on AlmaLinux/CentOS Docker) +RUN apk add --no-cache git || { sleep 2 && apk add --no-cache git; } WORKDIR /src COPY betterdesk-server/go.mod betterdesk-server/go.sum ./ @@ -38,7 +39,7 @@ WORKDIR /app # Build dependencies for native modules (better-sqlite3, bcrypt) # Note: sqlite-dev is NOT needed — better-sqlite3 bundles its own SQLite -RUN apk add --no-cache python3 make g++ +RUN apk add --no-cache python3 make g++ || { sleep 2 && apk add --no-cache python3 make g++; } COPY web-nodejs/package.json web-nodejs/package-lock.json* ./ RUN npm install --production @@ -53,14 +54,21 @@ LABEL maintainer="UNITRONIX" LABEL description="BetterDesk — All-in-One (Go Server + Node.js Console)" LABEL version="2.4.0" -# Install runtime packages +# Install runtime packages (retry for transient DNS failures) RUN apk add --no-cache \ ca-certificates \ curl \ sqlite \ tini \ supervisor \ - && mkdir -p /var/log/supervisor + && mkdir -p /var/log/supervisor \ + || { sleep 2 && apk add --no-cache \ + ca-certificates \ + curl \ + sqlite \ + tini \ + supervisor \ + && mkdir -p /var/log/supervisor; } # Create betterdesk user and directories RUN addgroup -S betterdesk && \ diff --git a/Dockerfile.console b/Dockerfile.console index c0007263..ed084bb3 100644 --- a/Dockerfile.console +++ b/Dockerfile.console @@ -11,7 +11,7 @@ WORKDIR /app # Build dependencies for native modules (better-sqlite3, bcrypt) # Note: sqlite-dev is NOT needed — better-sqlite3 bundles its own SQLite -RUN apk add --no-cache python3 make g++ +RUN apk add --no-cache python3 make g++ || { sleep 2 && apk add --no-cache python3 make g++; } # Copy package files first (better Docker cache) COPY web-nodejs/package.json web-nodejs/package-lock.json* ./ @@ -28,11 +28,12 @@ LABEL version="2.4.0" WORKDIR /app -# Install runtime dependencies +# Install runtime dependencies (retry for transient DNS failures) RUN apk add --no-cache \ sqlite \ curl \ tini \ + || { sleep 2 && apk add --no-cache sqlite curl tini; } \ && addgroup -S betterdesk \ && adduser -S -G betterdesk betterdesk diff --git a/Dockerfile.server b/Dockerfile.server index 4c823247..a88db30f 100644 --- a/Dockerfile.server +++ b/Dockerfile.server @@ -9,7 +9,7 @@ # ---- Build stage ---- FROM golang:1.25-alpine AS builder -RUN apk add --no-cache git +RUN apk add --no-cache git || { sleep 2 && apk add --no-cache git; } WORKDIR /src @@ -39,6 +39,8 @@ RUN apk add --no-cache \ curl \ sqlite \ tini \ + || { sleep 2 && apk add --no-cache \ + ca-certificates curl sqlite tini; } \ && addgroup -S betterdesk \ && adduser -S -G betterdesk betterdesk diff --git a/betterdesk-server/config/config.go b/betterdesk-server/config/config.go index e7a0dbc2..3961def9 100644 --- a/betterdesk-server/config/config.go +++ b/betterdesk-server/config/config.go @@ -3,6 +3,7 @@ package config import ( + "log" "net" "os" "strconv" @@ -229,6 +230,8 @@ func (c *Config) WSRelayPort() int { // GetRelayServers parses the comma-separated relay server list. // Ensures each entry includes a port; appends the default RelayPort if missing. +// Validates that each entry resolves to a valid IP or hostname (rejects single +// characters and obviously bogus values that would cause relay failures). func (c *Config) GetRelayServers() []string { if c.RelayServers == "" { return nil @@ -245,6 +248,14 @@ func (c *Config) GetRelayServers() []string { if _, _, err := net.SplitHostPort(s); err != nil { s = net.JoinHostPort(s, strconv.Itoa(c.RelayPort)) } + // Validate: extract the host part and reject obviously invalid entries + // (single letter, empty host, etc.) that would produce relay addresses + // like "a:21117" which cause all relay connections to fail. + host, _, err := net.SplitHostPort(s) + if err != nil || len(host) < 2 { + log.Printf("[config] WARNING: Ignoring invalid relay server %q (host too short or malformed)", s) + continue + } result = append(result, s) } return result diff --git a/betterdesk-server/signal/handler.go b/betterdesk-server/signal/handler.go index 32e3b167..565d2164 100644 --- a/betterdesk-server/signal/handler.go +++ b/betterdesk-server/signal/handler.go @@ -793,7 +793,18 @@ func (s *Server) handlePunchHoleSent(phs *pb.PunchHoleSent, senderAddr *net.UDPA // handleRequestRelay forwards relay setup request to target peer. func (s *Server) handleRequestRelay(msg *pb.RequestRelay, raddr *net.UDPAddr) { targetID := msg.Id - log.Printf("[signal] RequestRelay from %s for target %s (uuid=%s, secure=%v, connType=%v)", raddr, targetID, msg.Uuid, msg.Secure, msg.ConnType) + + // Generate UUID if the client sent an empty one. This happens when hole-punch + // fails after receiving PunchHoleResponse (which has no uuid field) and the + // client retries with RequestRelay. Without a valid UUID, the relay server + // rejects both connections. + relayUUID := msg.Uuid + if relayUUID == "" { + relayUUID = uuid.New().String() + log.Printf("[signal] RequestRelay: client %s sent empty UUID, generated %s", raddr, relayUUID[:8]) + } + + log.Printf("[signal] RequestRelay from %s for target %s (uuid=%s, secure=%v, connType=%v)", raddr, targetID, relayUUID, msg.Secure, msg.ConnType) target := s.peers.Get(targetID) relayServer := s.getRelayServer() @@ -841,7 +852,7 @@ func (s *Server) handleRequestRelay(msg *pb.RequestRelay, raddr *net.UDPAddr) { Union: &pb.RendezvousMessage_RelayResponse{ RelayResponse: &pb.RelayResponse{ SocketAddr: crypto.EncodeAddr(raddr), - Uuid: msg.Uuid, + Uuid: relayUUID, RelayServer: relayServer, Union: &pb.RelayResponse_Id{Id: msg.Id}, }, @@ -868,7 +879,7 @@ func (s *Server) handleRequestRelay(msg *pb.RequestRelay, raddr *net.UDPAddr) { resp := &pb.RendezvousMessage{ Union: &pb.RendezvousMessage_RelayResponse{ RelayResponse: &pb.RelayResponse{ - Uuid: msg.Uuid, + Uuid: relayUUID, RelayServer: relayServer, Union: &pb.RelayResponse_Pk{Pk: signedPk}, }, @@ -888,7 +899,15 @@ func (s *Server) handleRequestRelay(msg *pb.RequestRelay, raddr *net.UDPAddr) { // RelayResponse) caused timeouts for TCP signaling clients (e.g. logged-in users). func (s *Server) handleRequestRelayTCP(msg *pb.RequestRelay, raddr *net.UDPAddr) *pb.RendezvousMessage { targetID := msg.Id - log.Printf("[signal] RequestRelay (TCP) from %s for target %s (uuid=%s, secure=%v, connType=%v)", raddr, targetID, msg.Uuid, msg.Secure, msg.ConnType) + + // Generate UUID if the client sent an empty one (see handleRequestRelay comment). + relayUUID := msg.Uuid + if relayUUID == "" { + relayUUID = uuid.New().String() + log.Printf("[signal] RequestRelay (TCP): client %s sent empty UUID, generated %s", raddr, relayUUID[:8]) + } + + log.Printf("[signal] RequestRelay (TCP) from %s for target %s (uuid=%s, secure=%v, connType=%v)", raddr, targetID, relayUUID, msg.Secure, msg.ConnType) target := s.peers.Get(targetID) relayServer := s.getRelayServer() @@ -933,7 +952,7 @@ func (s *Server) handleRequestRelayTCP(msg *pb.RequestRelay, raddr *net.UDPAddr) Union: &pb.RendezvousMessage_RequestRelay{ RequestRelay: &pb.RequestRelay{ SocketAddr: crypto.EncodeAddr(raddr), - Uuid: msg.Uuid, + Uuid: relayUUID, Id: msg.Id, RelayServer: relayServer, Secure: msg.Secure, @@ -963,7 +982,7 @@ func (s *Server) handleRequestRelayTCP(msg *pb.RequestRelay, raddr *net.UDPAddr) return &pb.RendezvousMessage{ Union: &pb.RendezvousMessage_RelayResponse{ RelayResponse: &pb.RelayResponse{ - Uuid: msg.Uuid, + Uuid: relayUUID, RelayServer: relayServer, Union: &pb.RelayResponse_Pk{Pk: signedPk}, }, @@ -993,6 +1012,15 @@ func (s *Server) handleRelayResponseForward(msg *pb.RendezvousMessage, senderAdd return } + // If the target sent a RelayResponse with an empty UUID, both peers will fail + // to connect through relay. Generate a UUID as a last resort — the target may + // have already connected to relay with "" which won't pair, but at least this + // gives useful diagnostics and prevents silent failures. + if rr.Uuid == "" { + rr.Uuid = uuid.New().String() + log.Printf("[signal] WARNING: RelayResponse from %s has empty UUID — generated %s (target may have connected with empty UUID, relay pairing may fail)", senderAddr, rr.Uuid[:8]) + } + initiatorAddr, err := crypto.DecodeAddr(rr.SocketAddr) if err != nil { log.Printf("[signal] RelayResponse forward: cannot decode socket_addr: %v", err)