diff --git a/betterdesk-server/cdap/auth.go b/betterdesk-server/cdap/auth.go index 74d7bf87..e4ccd018 100644 --- a/betterdesk-server/cdap/auth.go +++ b/betterdesk-server/cdap/auth.go @@ -212,7 +212,7 @@ func (g *Gateway) authDeviceToken(p AuthPayload, clientIP string) (string, strin return "", "", fmt.Errorf("device token is not bound to this device") } peerInfo, err := g.db.GetPeer(p.DeviceID) - if err != nil || peerInfo == nil || peerInfo.Banned || peerInfo.SoftDeleted { + if err != nil || peerInfo == nil || peerInfo.Disabled || peerInfo.Banned || peerInfo.SoftDeleted { g.auditAction("cdap_auth_failed", clientIP, map[string]string{ "device_id": p.DeviceID, "reason": "device not enrolled or unavailable", diff --git a/betterdesk-server/cdap/auth_device_token_test.go b/betterdesk-server/cdap/auth_device_token_test.go index 2f649b55..5007c18b 100644 --- a/betterdesk-server/cdap/auth_device_token_test.go +++ b/betterdesk-server/cdap/auth_device_token_test.go @@ -80,3 +80,21 @@ func TestDeviceTokenCannotAuthenticateAnotherDevice(t *testing.T) { t.Fatal("device token bound to AGENT001 authenticated OTHER001") } } + +func TestDeviceTokenCannotAuthenticateDisabledDevice(t *testing.T) { + gateway, database, token := newDeviceTokenAuthGateway(t) + if err := database.UpsertPeer(&db.Peer{ + ID: "AGENT001", + Status: "ONLINE", + Disabled: true, + }); err != nil { + t.Fatal(err) + } + + if _, _, err := gateway.authDeviceToken(AuthPayload{ + Token: token, + DeviceID: "AGENT001", + }, "127.0.0.1"); err == nil { + t.Fatal("disabled device token unexpectedly authenticated") + } +} diff --git a/betterdesk-server/signal/handler.go b/betterdesk-server/signal/handler.go index a5d97858..0034400a 100644 --- a/betterdesk-server/signal/handler.go +++ b/betterdesk-server/signal/handler.go @@ -97,6 +97,22 @@ func (s *Server) targetAcceptsInboundSession(targetID string) bool { return state != db.PeerIDSoftDeleted } +// requiresRelayOnlyCompatibility keeps the temporary RustDesk-compatible +// support-agent path on relay transport. Direct transport has no equivalent +// server-bound session grant yet, so allowing P2P would create an +// authorization bypass around the passive-session policy. +func (s *Server) requiresRelayOnlyCompatibility(peerID string) bool { + if peerID == "" || s.db == nil { + return false + } + p, err := s.db.GetPeer(peerID) + if err != nil { + log.Printf("[signal] Compatibility peer %s lookup failed: %v", peerID, err) + return true + } + return isInboundOnlyPeer(p) +} + // handleUDPMessage dispatches a UDP message to the appropriate handler. func (s *Server) handleUDPMessage(msg *pb.RendezvousMessage, raddr *net.UDPAddr) { switch { @@ -737,8 +753,9 @@ func (s *Server) handlePunchHoleRequest(msg *pb.PunchHoleRequest, raddr *net.UDP return } - // Target is banned - if target.Banned { + // Target policy is durable; do not trust a stale live-peer entry after an + // administrator has disabled, banned, or removed the device. + if target.Banned || !s.targetAcceptsInboundSession(targetID) { resp := &pb.RendezvousMessage{ Union: &pb.RendezvousMessage_PunchHoleResponse{ PunchHoleResponse: &pb.PunchHoleResponse{ @@ -779,7 +796,9 @@ func (s *Server) handlePunchHoleRequest(msg *pb.PunchHoleRequest, raddr *net.UDP targetID, target.UDPAddr, target.StatusTier, time.Since(target.LastReg), relayServer) // If force relay or always use relay - if msg.ForceRelay || s.cfg.AlwaysUseRelay || hairpin || s.shouldForceRelayForPeers(initiatorID, targetID) { + if msg.ForceRelay || s.cfg.AlwaysUseRelay || hairpin || + s.shouldForceRelayForPeers(initiatorID, targetID) || + s.requiresRelayOnlyCompatibility(targetID) { log.Printf("[signal] PunchHole: force relay for %s", targetID) s.sendRelayResponse(target, raddr, msg, relayServer, initiatorID) return @@ -919,9 +938,9 @@ func (s *Server) handlePunchHoleRequestTCP(msg *pb.PunchHoleRequest, raddr *net. } } - // Target is banned — report as offline to initiator - if target.Banned { - log.Printf("[signal] PunchHole (TCP): target %s is banned, rejecting", targetID) + // Reject disabled, banned, or soft-deleted targets as offline. + if target.Banned || !s.targetAcceptsInboundSession(targetID) { + log.Printf("[signal] PunchHole (TCP): target %s is unavailable, rejecting", targetID) return &pb.RendezvousMessage{ Union: &pb.RendezvousMessage_PunchHoleResponse{ PunchHoleResponse: &pb.PunchHoleResponse{ @@ -959,7 +978,9 @@ func (s *Server) handlePunchHoleRequestTCP(msg *pb.PunchHoleRequest, raddr *net. // PunchHoleResponse), generate their own UUID, and connect to relay with it // — while the target connects with the server's UUID. This broke relay // pairing every time (Issue #66). - if msg.ForceRelay || s.cfg.AlwaysUseRelay || hairpin || s.shouldForceRelayForPeers(initiatorID, targetID) { + if msg.ForceRelay || s.cfg.AlwaysUseRelay || hairpin || + s.shouldForceRelayForPeers(initiatorID, targetID) || + s.requiresRelayOnlyCompatibility(targetID) { log.Printf("[signal] PunchHole (TCP): force relay for %s (returning SYMMETRIC to let client drive relay UUID)", targetID) var signedPk []byte @@ -1238,9 +1259,9 @@ func (s *Server) handleRequestRelay(msg *pb.RequestRelay, raddr *net.UDPAddr) { return } - // Target is banned — reject relay as if offline - if target.Banned { - log.Printf("[signal] RequestRelay: target %s is banned, rejecting", targetID) + // Reject disabled, banned, or soft-deleted targets as offline. + if target.Banned || !s.targetAcceptsInboundSession(targetID) { + log.Printf("[signal] RequestRelay: target %s is unavailable, rejecting", targetID) resp := &pb.RendezvousMessage{ Union: &pb.RendezvousMessage_RelayResponse{ RelayResponse: &pb.RelayResponse{ @@ -1413,9 +1434,9 @@ func (s *Server) handleRequestRelayTCP(msg *pb.RequestRelay, raddr *net.UDPAddr, } } - // Target is banned — reject relay as if offline - if target.Banned { - log.Printf("[signal] RequestRelay (TCP): target %s is banned, rejecting", targetID) + // Reject disabled, banned, or soft-deleted targets as offline. + if target.Banned || !s.targetAcceptsInboundSession(targetID) { + log.Printf("[signal] RequestRelay (TCP): target %s is unavailable, rejecting", targetID) return &pb.RendezvousMessage{ Union: &pb.RendezvousMessage_RelayResponse{ RelayResponse: &pb.RelayResponse{ diff --git a/betterdesk-server/signal/handler_test.go b/betterdesk-server/signal/handler_test.go index 17fa8a69..4618e4ba 100644 --- a/betterdesk-server/signal/handler_test.go +++ b/betterdesk-server/signal/handler_test.go @@ -1029,6 +1029,83 @@ func TestInboundOnlyAgentCanBeConnectionTarget(t *testing.T) { } } +func TestInboundOnlySupportTargetForcesRelay(t *testing.T) { + srv, database := newTestSignalServer(t, config.EnrollmentModeOpen) + if err := database.UpsertPeer(&db.Peer{ID: "CLIENTRELAY1", DeviceType: "desktop"}); err != nil { + t.Fatal(err) + } + if err := database.UpsertPeer(&db.Peer{ + ID: "SUPPORTRELAY1", + DeviceType: "os_agent", + Tags: "support-agent", + }); err != nil { + t.Fatal(err) + } + putOnlinePeer(srv, "CLIENTRELAY1", "198.51.100.160", 51000, peer.ConnTCP) + putOnlinePeer(srv, "SUPPORTRELAY1", "203.0.113.160", 52000, peer.ConnTCP) + + response := srv.handlePunchHoleRequestTCP( + &pb.PunchHoleRequest{Id: "SUPPORTRELAY1"}, + udpAddr("198.51.100.160", 51000), + ) + punch := response.GetPunchHoleResponse() + if punch == nil { + t.Fatalf("response = %+v, want PunchHoleResponse", response) + } + if punch.GetNatType() != pb.NatType_SYMMETRIC { + t.Fatalf("NatType = %v, want forced relay (%v)", punch.GetNatType(), pb.NatType_SYMMETRIC) + } +} + +func TestUnavailableTargetCannotReceiveRelay(t *testing.T) { + for _, tc := range []struct { + name string + setup func(t *testing.T, database db.Database, targetID string) + }{ + { + name: "disabled", + setup: func(t *testing.T, database db.Database, targetID string) { + t.Helper() + if err := database.UpsertPeer(&db.Peer{ID: targetID, Disabled: true}); err != nil { + t.Fatal(err) + } + }, + }, + { + name: "soft deleted", + setup: func(t *testing.T, database db.Database, targetID string) { + t.Helper() + if err := database.UpsertPeer(&db.Peer{ID: targetID}); err != nil { + t.Fatal(err) + } + if err := database.DeletePeer(targetID); err != nil { + t.Fatal(err) + } + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + srv, database := newTestSignalServer(t, config.EnrollmentModeOpen) + const initiatorID = "ACTIVEINIT1" + const targetID = "UNAVAILTGT1" + if err := database.UpsertPeer(&db.Peer{ID: initiatorID, DeviceType: "desktop"}); err != nil { + t.Fatal(err) + } + tc.setup(t, database, targetID) + putOnlinePeer(srv, initiatorID, "198.51.100.150", 51000, peer.ConnTCP) + putOnlinePeer(srv, targetID, "203.0.113.150", 52000, peer.ConnTCP) + + response := srv.handleRequestRelayTCP(&pb.RequestRelay{ + Id: targetID, + Uuid: "unavailable-target-" + strings.ReplaceAll(tc.name, " ", "-"), + }, udpAddr("198.51.100.150", 51000), peer.ConnTCP) + if relay := response.GetRelayResponse(); relay == nil || relay.RefuseReason != "Target offline" { + t.Fatalf("relay response = %+v, want unavailable target rejection", response) + } + }) + } +} + func TestPanelProxyLoopbackCanPunchHoleWithoutPeer(t *testing.T) { srv, _ := newTestSignalServer(t, config.EnrollmentModeManaged) putOnlinePeer(srv, "TGTWEB1", "203.0.113.90", 52000, peer.ConnTCP) diff --git a/betterdesk-support-agent/apihttp.go b/betterdesk-support-agent/apihttp.go index 18127d7b..3e3d4304 100644 --- a/betterdesk-support-agent/apihttp.go +++ b/betterdesk-support-agent/apihttp.go @@ -3,7 +3,11 @@ package main import ( "bytes" "context" + "crypto/sha256" + "crypto/subtle" "crypto/tls" + "crypto/x509" + "encoding/hex" "encoding/json" "fmt" "io" @@ -25,15 +29,62 @@ func tlsInsecureEnabled() bool { // apiHTTPClient returns an HTTP client for BetterDesk API calls. func apiHTTPClient(timeout time.Duration) *http.Client { - client := &http.Client{Timeout: timeout} - if tlsInsecureEnabled() { - client.Transport = &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // opt-in dev - } + pin := "" + if b := GetBranding(); b.Server != nil { + pin = b.Server.CertPin } + return apiHTTPClientWithPin(timeout, pin) +} + +func apiHTTPClientWithPin(timeout time.Duration, pin string) *http.Client { + client := &http.Client{Timeout: timeout} + pin = normalizeServerCertPin(pin) + if pin == "" && !tlsInsecureEnabled() { + return client + } + + tlsConfig := &tls.Config{MinVersion: tls.VersionTLS12} + if pin != "" { + // Pinning the leaf SPKI is an authentication check stronger than + // platform trust alone. The profile's endpoint allowlist prevents this + // key from being used for an arbitrary destination. + tlsConfig.InsecureSkipVerify = true //nolint:gosec // verified below + tlsConfig.VerifyPeerCertificate = func(rawCerts [][]byte, _ [][]*x509.Certificate) error { + if len(rawCerts) == 0 { + return fmt.Errorf("tls: server presented no certificate") + } + leaf, err := x509.ParseCertificate(rawCerts[0]) + if err != nil { + return fmt.Errorf("tls: parse leaf certificate: %w", err) + } + sum := sha256.Sum256(leaf.RawSubjectPublicKeyInfo) + got := hex.EncodeToString(sum[:]) + if subtle.ConstantTimeCompare([]byte(got), []byte(pin)) != 1 { + return fmt.Errorf("tls: server public-key pin mismatch") + } + return nil + } + } else { + // Development-only self-signed test mode. tlsInsecureEnabled() cannot + // become true in a release build. + tlsConfig.InsecureSkipVerify = true //nolint:gosec // opt-in development mode + } + client.Transport = &http.Transport{TLSClientConfig: tlsConfig} return client } +func normalizeServerCertPin(pin string) string { + pin = strings.ToLower(strings.TrimSpace(pin)) + pin = strings.NewReplacer("sha256:", "", ":", "", " ", "", "\t", "", "\n", "").Replace(pin) + if len(pin) != sha256.Size*2 { + return "" + } + if _, err := hex.DecodeString(pin); err != nil { + return "" + } + return pin +} + // apiBaseURL resolves the Go server API base (…/api) from branding. func apiBaseURL(b Branding) string { if b.Server != nil && strings.TrimSpace(b.Server.APIURL) != "" { diff --git a/betterdesk-support-agent/apihttp_test.go b/betterdesk-support-agent/apihttp_test.go index 442caccc..01080723 100644 --- a/betterdesk-support-agent/apihttp_test.go +++ b/betterdesk-support-agent/apihttp_test.go @@ -1,7 +1,11 @@ package main import ( + "crypto/sha256" + "encoding/hex" "net/http" + "net/http/httptest" + "strings" "testing" "time" ) @@ -40,3 +44,33 @@ func TestHTTPGetKeepsVerificationEnabledUnlessDevelopmentOptIn(t *testing.T) { t.Fatal("development health probe did not configure insecure TLS") } } + +func TestAPIHTTPClientWithPinVerifiesServerSPKI(t *testing.T) { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + sum := sha256.Sum256(server.Certificate().RawSubjectPublicKeyInfo) + pin := hex.EncodeToString(sum[:]) + + response, err := apiHTTPClientWithPin(time.Second, pin).Get(server.URL) + if err != nil { + t.Fatalf("pinned request failed: %v", err) + } + _ = response.Body.Close() + + if _, err := apiHTTPClientWithPin(time.Second, strings.Repeat("0", 64)).Get(server.URL); err == nil { + t.Fatal("mismatched server pin unexpectedly succeeded") + } +} + +func TestNormalizeServerCertPin(t *testing.T) { + pin := "sha256:AA:BB " + strings.Repeat("0", 60) + if got := normalizeServerCertPin(pin); got != "aabb"+strings.Repeat("0", 60) { + t.Fatalf("normalized pin = %q", got) + } + if got := normalizeServerCertPin("not-a-pin"); got != "" { + t.Fatalf("invalid pin normalized to %q", got) + } +} diff --git a/betterdesk-support-agent/branding.go b/betterdesk-support-agent/branding.go index 8fc2f092..c9b59801 100644 --- a/betterdesk-support-agent/branding.go +++ b/betterdesk-support-agent/branding.go @@ -213,6 +213,9 @@ func (b Branding) validateReleaseProfile(now time.Time) error { if !allSecureAndAllowed(b.AllowedEndpoints, b.Server.Address, b.Server.APIURL, b.Server.CDAPURL) { return fmt.Errorf("release branding profile has unauthorized endpoint") } + if b.Server.CertPin != "" && normalizeServerCertPin(b.Server.CertPin) == "" { + return fmt.Errorf("release branding profile has invalid certificate pin") + } return nil } diff --git a/betterdesk-support-agent/branding_profile_test.go b/betterdesk-support-agent/branding_profile_test.go index 028900e4..a1cc6a14 100644 --- a/betterdesk-support-agent/branding_profile_test.go +++ b/betterdesk-support-agent/branding_profile_test.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "fmt" "testing" + "time" "github.com/unitronix/betterdesk-support-agent/internal/brandprofile" ) @@ -47,6 +48,10 @@ func TestReleaseBrandingRequiresValidSignedProfile(t *testing.T) { branding.Server.Address != "https://support.example.test" { t.Fatalf("unexpected branding: %+v", branding) } + branding.Server.CertPin = "invalid-pin" + if err := branding.validateReleaseProfile(time.Now()); err == nil { + t.Fatal("release profile unexpectedly accepted an invalid certificate pin") + } if _, err := decodeBrandingProfile(profile, []byte(publicKeyResource), true); err == nil { t.Fatal("release profile unexpectedly accepted unsigned branding") diff --git a/betterdesk-support-agent/netcheck.go b/betterdesk-support-agent/netcheck.go index f706fe98..08844fd8 100644 --- a/betterdesk-support-agent/netcheck.go +++ b/betterdesk-support-agent/netcheck.go @@ -2,7 +2,6 @@ package main import ( "context" - "crypto/tls" "encoding/json" "fmt" "net/http" @@ -135,11 +134,12 @@ func httpGet(endpoint string) ([]byte, time.Duration, error) { } func healthHTTPClient(endpoint string) *http.Client { - client := &http.Client{Timeout: 8 * time.Second} - if strings.HasPrefix(endpoint, "https://") && tlsInsecureEnabled() { - client.Transport = &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // opt-in dev only - } + if !strings.HasPrefix(strings.ToLower(endpoint), "https://") { + return &http.Client{Timeout: 8 * time.Second} } - return client + pin := "" + if b := GetBranding(); b.Server != nil { + pin = b.Server.CertPin + } + return apiHTTPClientWithPin(8*time.Second, pin) }